Add configurable character limits and feature toggles for polls#6435
Add configurable character limits and feature toggles for polls#6435ryanhurststrava wants to merge 4 commits into
Conversation
Introduces PollsConfig to control poll feature availability and enforce character limits on questions and options. Poll features (multiple votes, anonymous voting, suggest options, add comments) can now be hidden or preset with default values through ChatUI.pollsConfig or passed directly to CreatePollDialogFragment.
WalkthroughThis PR introduces a centralized poll configuration system allowing apps to control which poll creation features are user-configurable, their default states, and text input limits. New ChangesPoll Configuration Feature
Poem
🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/attachment/picker/poll/CreatePollDialogFragment.kt (1)
240-240: ⚡ Quick winRemove unnecessary cast.
The explicit cast to
android.os.Parcelableis redundant sincePollsConfigalready implementsParcelable(defined at line 70 in PollsConfig.kt). Kotlin's type inference handles this automatically.Simplified code
- pollsConfig?.let { config -> putParcelable(ARG_POLLS_CONFIG, config as android.os.Parcelable) } + pollsConfig?.let { putParcelable(ARG_POLLS_CONFIG, it) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/attachment/picker/poll/CreatePollDialogFragment.kt` at line 240, Remove the redundant explicit cast in the putParcelable call: where CreatePollDialogFragment stores pollsConfig with putParcelable(ARG_POLLS_CONFIG, config as android.os.Parcelable), change it to pass pollsConfig (config) directly since PollsConfig already implements Parcelable; update the call in the block that references pollsConfig and ARG_POLLS_CONFIG to remove "as android.os.Parcelable".stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/attachment/picker/poll/PollsConfig.kt (1)
67-67: ⚡ Quick winConsider renaming
addCommentstoallowAnswersfor consistency.The config property is named
addComments, but the underlying model field inCreatePollViewModel(line 49) andPollConfig(line 106 in CreatePollViewModel.kt) is namedallowAnswers. This naming mismatch may confuse developers trying to understand how the config maps to the actual poll behavior.Suggest renaming to maintain consistent terminology across the configuration and implementation layers.
Proposed fix
- val addComments: PollsEntryConfig = PollsEntryConfig.Default, + val allowAnswers: PollsEntryConfig = PollsEntryConfig.Default,Update corresponding documentation and usage in
CreatePollDialogFragment.ktlines 124-129.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/attachment/picker/poll/PollsConfig.kt` at line 67, Rename the PollsConfig property addComments to allowAnswers to match the field name used in CreatePollViewModel and PollConfig; update the property declaration in PollsConfig (currently val addComments: PollsEntryConfig = PollsEntryConfig.Default) to val allowAnswers: PollsEntryConfig = PollsEntryConfig.Default, then update all usages and bindings (including where CreatePollDialogFragment populates the config) and any documentation/comments to use allowAnswers so terminology is consistent across PollsConfig, CreatePollViewModel, and PollConfig.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/attachment/picker/poll/OptionsAdapter.kt`:
- Around line 30-40: The public API changed because the primary constructor
parameter order for OptionsAdapter now places optionTextLimit first, breaking
existing positional calls; restore a backward-compatible single-argument
constructor by adding a public constructor matching the old signature
(constructor(onOptionChange: (id: Int, text: String) -> Unit)) that delegates to
the new primary (this(null, onOptionChange)), mark it `@Deprecated` with a message
pointing to the new constructor/parameter and add `@JvmOverloads/`@JvmName if
needed for Java callers so existing binaries still resolve the single-arg
constructor while signaling callers to move to the new API.
---
Nitpick comments:
In
`@stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/attachment/picker/poll/CreatePollDialogFragment.kt`:
- Line 240: Remove the redundant explicit cast in the putParcelable call: where
CreatePollDialogFragment stores pollsConfig with putParcelable(ARG_POLLS_CONFIG,
config as android.os.Parcelable), change it to pass pollsConfig (config)
directly since PollsConfig already implements Parcelable; update the call in the
block that references pollsConfig and ARG_POLLS_CONFIG to remove "as
android.os.Parcelable".
In
`@stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/attachment/picker/poll/PollsConfig.kt`:
- Line 67: Rename the PollsConfig property addComments to allowAnswers to match
the field name used in CreatePollViewModel and PollConfig; update the property
declaration in PollsConfig (currently val addComments: PollsEntryConfig =
PollsEntryConfig.Default) to val allowAnswers: PollsEntryConfig =
PollsEntryConfig.Default, then update all usages and bindings (including where
CreatePollDialogFragment populates the config) and any documentation/comments to
use allowAnswers so terminology is consistent across PollsConfig,
CreatePollViewModel, and PollConfig.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0ab5b5db-0f77-4ecc-a22d-aa08094ec362
📒 Files selected for processing (5)
stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/ChatUI.ktstream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/attachment/picker/poll/CreatePollDialogFragment.ktstream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/attachment/picker/poll/CreatePollViewModel.ktstream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/attachment/picker/poll/OptionsAdapter.ktstream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/attachment/picker/poll/PollsConfig.kt
| createPollViewModel.setAllowMultipleVotes(pollsConfig.multipleVotes.defaultValue) | ||
| binding.multipleAnswersLabel.isVisible = pollsConfig.multipleVotes.configurable | ||
| binding.multipleAnswersSwitch.isVisible = pollsConfig.multipleVotes.configurable | ||
| if (pollsConfig.multipleVotes.configurable) { |
There was a problem hiding this comment.
I think we're introducing a new edge case here.
If we have multipleVotes.defaultValue = true + multipleVotes.configurable = false, then we end up in a state where:
- In the VM,
allowMultipleVotes = truesince we set it here multipleAnswersCountis hidden because ofconfigurable = falsesendMenuItemis disabled because of the above (due to this logic) so the user can never create the poll
In Compose v7 we changed the behavior to allow multipleVotes = true + maxVotes = null which means unlimited votes, so maybe we could do something like that? It's a behavior change, though.
Another option is to disallow the problematic combo, e.g. we could add a check(multipleVotes.configurable == true || multipleVotes.defaultValue == false) call in PollsConfig's constructor.
cc @andremion @VelikovPetar wdyt?
There was a problem hiding this comment.
I am more in favour of disallowing the problematic combination - I wouldn't want to change the default behaviour in a minor release. But once we port this addition to V7, I would say to align it with the compose implementation.
There was a problem hiding this comment.
Can you be more explicit at what you'd like to see here?
another option would be to remove the multiple votes configuration from this pr. I included it for completeness, but it is not a feature my team needs.
There was a problem hiding this comment.
I'd also leave it included for completeness. Basically the suggestion is to put this inside PollsConfig constructor:
init {
require(multipleVotes.configurable || !multipleVotes.defaultValue) {
"Invalid PollsConfig: multipleVotes cannot have defaultValue=true while " +
"configurable=false as the user would be unable to set maxVotesAllowed."
}
}
| binding.addACommentLabelSwitch.setOnCheckedChangeListener { _, isChecked -> | ||
| createPollViewModel.setAllowAnswers(isChecked) | ||
| } |
There was a problem hiding this comment.
I added this, but it is more or less unrelated to this pr. I think the current functionality(before this pr) is that this switch doesn't do anything. I wasn't able to get this to work in my testing with the changes in this pr either though, so I'm curious what your thoughts are.
There was a problem hiding this comment.
It definitely looks like we had a bug both on the creation side and the consuming side, since we never read "allow answers" when displaying poll messages. We'll check that on our side, thanks for fixing the problem at creation time!
ryanhurststrava
force-pushed
the
feature/poll_character_limits_v2
branch
from
c698547 to
a20a5d8
Compare
|
We have some failing static checks:
Could you fix them? 🙏 |
…y breaking the long comment line 2. Spotless - Applied formatting fixes (added license header to PollFeatureConfig.kt) 3. API Check - Regenerated the API dump file
3 participants
🎯 Goal
Introduce comprehensive configuration for poll creation features, allowing developers to control which features are available to users, set default values, and enforce character limits on questions and options. This provides flexibility for different use cases where certain poll features may need to be restricted, pre-configured, or character-limited based on UI constraints, business rules, or localization requirements. This loosely follows the iOS implementation.
🛠 Implementation
New Files Created
PollsConfig.ktPollsConfigdata class for configuring all poll creation featuresPollsEntryConfigdata class for individual feature configuration (configurable flag + default value)multipleVotes- Allow users to select multiple optionsanonymousPoll- Hide voter identitiessuggestAnOption- Allow users to add their own optionsaddComments- Allow users to add comments to pollsquestionTextLimit- Optional character limit for poll questionsoptionTextLimit- Optional character limit for poll optionsParcelablefor passing via Bundle argumentsDefaultandNotConfigurablepresetsModified Files
ChatUI.ktpollsConfigproperty to provide global default configurationPollsConfig.Default(all features configurable and disabled by default)CreatePollDialogFragment.ktnewInstance()to accept optionalPollsConfigparameterChatUI.pollsConfigwhen not providedconfigurePollFeatures()method to show/hide UI elements based on configInputFilter.LengthFilterCreatePollViewModel.ktallowAnswersfield andsetAllowAnswers()methodallowAnswersto the poll configuration when creating pollsOptionsAdapter.ktoptionTextLimitparameter to constructorInputFilter.LengthFilterto option EditText fields when limit is specifiedKey Design Decisions
ChatUI.pollsConfigor per-dialog vianewInstance()null(unlimited) unless explicitly specifiedUsage Examples
📱 UI Changes
✅ Testing
Manual Testing Steps
configurable = false, verify UI elements are hiddenChatUI.pollsConfigvs per-dialog confignewInstance()callsExpected Behavior
Contributor Checklist
developbranchReviewer Checklist
🎉 GIF
1778192411.mp4
Screen_recording_20260511_173314.webm
Summary by CodeRabbit
Release Notes